Guard ProjectionExec::try_pushdown_sort against stale name/index metadata#156
Open
Jeadie wants to merge 5 commits into
Open
Guard ProjectionExec::try_pushdown_sort against stale name/index metadata#156Jeadie wants to merge 5 commits into
ProjectionExec::try_pushdown_sort against stale name/index metadata#156Jeadie wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens sort pushdown through ProjectionExec by preventing rewrite/pushdown when a Column’s name doesn’t match the projection output alias at the same index (avoiding incorrect pushdown when column metadata is stale or mismatched).
Changes:
- Add a name-vs-alias guard in
ProjectionExec::try_pushdown_sortto reject pushdown whenColumn(name, index)doesn’t match the projection’s output alias atindex. - Add a regression test ensuring sort pushdown does not occur when the sort column’s name/index metadata is inconsistent with the projection output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
datafusion/physical-plan/src/projection.rs |
Adds a safety check to avoid rewriting sort expressions through projections when column metadata is mismatched. |
datafusion/core/tests/physical_optimizer/pushdown_sort.rs |
Adds a regression test snapshot covering the name/index mismatch scenario to ensure no pushdown occurs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Jeadie
changed the title
ProjectionExec::try_pushdown_sort against stale name/index metadata
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a safety check in ProjectionExec::try_pushdown_sort to prevent incorrect sort pushdown when a sort column’s (name, index) metadata is stale or mismatched.
ProjectionExectranslated parent sort requirements to child columns using index-only mapping:col@iprojection.expr()[i]If optimizer rewrites left a stale pair (e.g. name
_score, index3, but projection slot 3 is actuallypicked_at), pushdown could incorrectly rewrite_score@3intopicked_at@3, causing invalid ordering assumptions and potentialSanityCheckPlanfailures.